Skip to content

fix(driver-sql): the hash-shadow arm survives a plain unique over duplicate rows - #16288

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15479-mysql-hash-shadow-plain-unique
Sep 6, 2026
Merged

fix(driver-sql): the hash-shadow arm survives a plain unique over duplicate rows#16288
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15479-mysql-hash-shadow-plain-unique

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #15479

syncDeclaredIndexes handles a declared unique the database refuses in one catch, and that catch has two arms: the DIRECT one, and the hash-shadow one MySQL takes when a key part exceeds the 768-char utf8mb4 ceiling (#11627). #14902 / PR #15477 brought the direct arm to the ADR-0120 D4 disposition — a uniqueness violation over existing rows is a durability degradation, not a fatal. The shadow arm kept the older guard, nullSafe.size > 0 && isUniqueViolationError(shadowErr), so a PLAIN unique (tenancy: { enabled: false }, or an explicit unique: 'global') matched neither branch and fell through to the unkeyable-column refusal, which throws.

The card said this was unmeasurable here. That was the one thing it got wrong, and correcting it is most of this PR

The card filed the gap rather than a fix because "the dispatch container has no MySQL", and was explicit that its reachability claim was "derived by reading the code, not measured". That is a good instinct on a durability path and it should stay cheap to file — but the constraint was not real. MySQL 8.0.46 installs from the distro archive in this container (apt-get install mysql-server, mysqld started directly since there is no systemd), and every reading below is against that live server. No Docker daemon is running here; none was needed.

The premise is therefore no longer derived. It is measured, and it reproduces.

Before — the boot dies, and its advice is actively wrong

On the unmodified tree, with two rows already sharing a 900-char value and a plain unique newly declared over them, initObjects throws:

[sql-driver] cannot create index 'uniq_os15479_plain_v' on "os15479_plain" — MySQL refuses a
TEXT/BLOB column in a key without a key length. Column(s) "v" are stored as TEXT because the
field declares no `maxLength` (or one wider than 768 characters …). Declare `maxLength` on the
field(s) so the column is emitted as varchar(n) and can be keyed (#11374).
  cause: ER_BLOB_KEY_WITHOUT_LENGTH (errno 1170)

Note what the operator is told: declare a maxLength — on a field that already declares maxLength: 1024. The message is about the wrong problem entirely. The actual cause is two duplicate rows, and it names neither them nor a remedy. That is the shape #14902 graded p1 on the sibling arm.

After — the three acceptance conditions from the card

Same fixture, same server, after the change — this line is quoted verbatim from a live run, not composed here:

[sql-driver] cannot create hash-shadow unique index 'uniq_zz15479_v' on "zz15479" — existing
rows violate it. Conflicting group(s): (v="ppp…ppp") × 2 rows. The constraint 'v' is NOT
enforced until the data is deduplicated: run "os migrate plan" for the conflicting rows.
  • the boot survives (initObjects resolves);
  • the durability log names the conflicting group and the remedy;
  • the index is absent afterwards, and the atomic ALTER left no orphaned shadow column (both read back from information_schema, never from our own DDL).

Not a guard widening — the two-arm message split

The surviving branch's message said the rows "violate the NULL-safe key (duplicates the previous void constraint admitted, #5030)". Neither clause is true of a plain unique: nothing admitted these rows, and there is no NULL-safe key. Widening the guard while leaving one message would ship a factually false durability log — worse than the throw it replaces, because it sends the operator hunting a NULL-distinct index that never existed and an #5030 defect that never occurred.

So the arms are split. The NULL-safe branch keeps its wording untouched; the plain branch carries the direct arm's already-reviewed sentence verbatim, and reuses formatDuplicateGroups — the module-local helper #14902 added precisely so the two sites reporting a blocked unique cannot drift into two spellings. The only word that is this arm's own is the route noun "hash-shadow", because that is what honestly names which physical route was attempted, and the NULL-safe branch above already spells it that way.

The uniqueness limb comes from the enclosing if (unique) a few lines above, so the new guard asks only the violation question. Verified that the enclosing condition really is unique at that nesting level rather than assumed.

Measurements

Environment: MySQL 8.0.46-0ubuntu0.24.04.4, started directly (mysqld --bind-address=127.0.0.1), reached over TCP through OS_TEST_MYSQL_URL and this package's per-file schema isolation (#9350). All runs at 895d55979.

Firing control, taken FIRST, on the unmodified tree — without it a green run afterwards proves nothing. The two existing opt-in cells named on the card: 15 passed, 1 skipped. The single skip is the un-provisioned Postgres cell, not a MySQL one — 12 live-MySQL tests genuinely executed and drove real DDL.

Red, then green. The new cell sql-driver-15479-shadow-plain-unique-duplicates.test.ts on the unmodified source: 2 failed / 2 passed — the two "boot survives" blocks failed with the ER_BLOB_KEY_WITHOUT_LENGTH throw quoted above. After the change, with both existing cells and #14902's suite alongside: 27 passed, 1 skipped (4 files), same lone Postgres skip.

The new cell carries four blocks, two of which exist to catch the plausible wrong fixes: a CONTROL asserting the NULL-safe arm still says the NULL-safe thing (a single shared message fails here), and a positive control that a plain shadow unique over CLEAN data is still created and still enforces (a guard that turned the shadow route off fails here). Both shapes that reach the plain path are covered, because a guard keyed on only one of them would pass locally and fail in production.

Whole package, no live URL: 155 files passed / 10 skipped, 2389 tests passed, 141 skipped. typecheck green — and proven to actually cover the change rather than excluding tests: tsc --listFiles puts both edited files in the 577-file checked set.

Published surface (clause ②): no, measured by ablation rather than judged. Built at head, swapped sql-driver.ts back to the merge base dacb73f4f, rebuilt, compared every declaration file the package's files[] publishes (dist/index.d.ts, dist/index.d.mts), then restored — restore proven by blob equality against the HEAD blob and an empty git diff HEAD. Exported names 73 → 73, class/interface members 349 → 349, nothing added, nothing removed. The bytes do differ, so the delta is classified rather than waved through: one hunk, 6 added lines, 0 removed, all six JSDoc comment lines, with the declaration protected syncDeclaredIndexes(…): Promise<void>; byte-identical on both sides. The instrument was then proven live against a change that SHOULD move the surface — a throwaway protected zzControlProbe15479 moved members 349 → 350 and was named by the extractor — so the 349 → 349 reading is a measurement, not a blind spot.

Gates: the 54 families derived by scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack from the real change set (the script's own three-dot derivation, no hand-built file list). 53 ran, all green at 895d55979; check:partof-closing-keyword reads PR_BODY from the environment and is run against this posted body rather than an empty one. Three of them (check:published-readme-exports, check:dual-build-cjs-loads, check:type-check-debt) first returned their explicit exit 3 — "nothing was measured" — so the full 71-package closure was built and all three re-run to a real green. And because a gate's green can be about a population that excludes your package, check:dts-closure was controlled: removing driver-sql/dist/index.d.ts turns it red naming @objectstack/driver-sql, restoring it (byte-identical) turns it green.

Out of scope

os migrate plan is untouched — #15477's D4 pre-flight is guard-free and already reports this op destructive with the row report.

Changeset: patch on @objectstack/driver-sql. Behaviour on every other dialect is unchanged; SQLite and Postgres never refuse the direct index and so never reach this arm.

This PR was produced by the domain:engine execution seat in session session_01ARYe3yQTQCUFm5qPYNgKaJ, on branch claude/issue-15479-mysql-hash-shadow-plain-unique.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ


Generated by Claude Code

…licate rows

`syncDeclaredIndexes`'s `catch` handles a refused unique on two arms: the
direct one, and the hash-shadow one MySQL takes when a key part exceeds the
768-char utf8mb4 ceiling. #14902 brought the direct arm to the ADR-0120 D4
disposition -- a uniqueness violation over existing rows is a durability
degradation, not a fatal. The shadow arm still required a NULL-safe
organization key part as well, so a PLAIN unique matched neither branch, fell
through to the unkeyable-column refusal and took the boot down.

Measured on live MySQL 8.0.46: the boot died with ER_BLOB_KEY_WITHOUT_LENGTH,
advising a `maxLength` the field already declared, naming neither the two
duplicate rows nor a remedy.

Not a bare guard widening. The surviving branch's message says the rows violate
the NULL-safe key and duplicate what the previous void constraint admitted
(#5030); neither clause is true of a plain unique. The two arms are split so
the NULL-safe one keeps its wording and the plain one carries the direct arm's
reviewed sentence.

Verified on live MySQL 8.0.46 through a new opt-in cell, with the two existing
shadow cells as the firing control (green before and after).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 2 documentable anchor(s).

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/data-modeling/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/permissions/tenant-audit-census.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol, a top-level class))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via SqlDriver (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json bda7b7c83b82c09f229ffd613c6900200089189apackageMentionDocs.

Which tree this was computed on

This run read content/docs from 120dee100e6dffa686e89cdd97160dbf040b95d8 — the merge of head 895d55979f255bb0c5da6469553f9095e86cb0a2 into base bda7b7c83b82c09f229ffd613c6900200089189a, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 120dee100e6dffa686e89cdd97160dbf040b95d8 && git checkout 120dee100e6dffa686e89cdd97160dbf040b95d8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin bda7b7c83b82c09f229ffd613c6900200089189a 895d55979f255bb0c5da6469553f9095e86cb0a2 && git checkout -B drift-repro bda7b7c83b82c09f229ffd613c6900200089189a && git merge --no-ff 895d55979f255bb0c5da6469553f9095e86cb0a2

node scripts/docs-audit/affected-docs.mjs --json bda7b7c83b82c09f229ffd613c6900200089189a

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs bda7b7c83b82c09f229ffd613c6900200089189a → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

driver-sql: the MySQL hash-shadow arm still takes the boot down on a PLAIN unique over duplicate rows — the same guard #14902 moved on the direct arm

2 participants